-
-
Notifications
You must be signed in to change notification settings - Fork 243
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add intergrated Help Features #2088
Conversation
ba85432
to
75bff32
Compare
Can you please rebase your PR? Your commit history is messed up because of the merge ... |
I'll give it a try when I get a chance later. As far as I know, it's messed up partially because I'm still learning all this rebase stuff and somehow messed it up the first time. Edit: Did I figure it out? It looks like I figured it out. |
529e81c
to
e047d39
Compare
Job #1257: Bundle Size — 15.8MiB (+0.26%).Important Bundle introduced 1 and removed 1 duplicate package – View changed duplicate packages Warning Bundle introduced 13 new packages: @jsep-plugin/regex, @jsep-plugin/arrow, @jsep-plugin/object and 10 more – View changed packages Bundle metrics
Bundle size by type
View job #1257 report View JustinGeorgi:jag-help-doc branch activity |
e047d39
to
6c80f13
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this @JustinGeorgi!
I will test it asap, one initial remark though:
<f7-link v-if="$store.state.developerDock" icon-f7="question_circle_fill" @click="$f7.emit('toggleDeveloperDock')" /> | ||
<f7-link v-else icon-f7="question_circle" @click="$f7.emit('selectDeveloperDock',{'dock':'help','helpTab':'current'})" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about when the developer dock can't be displayed because the screen is too narrow?
This new button/link will still be displayed but appear to (and in fact will) do nothing I think, which is not good.
Maybe best remove them when the current screen is not able to display the dock - or keep them but just link them to relevant documentation instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh yeah, forgot that point from the previous discussion. If it's all the same to you, I'm inclined to just remove it so it doesn't clutter the navbar on narrower screens.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do think it's best to either remove it, or perhaps change it to a regular link to the documentation when the developer dock wouldn't fit.
By the way, I think when those icons make their way in the navbar, we should replace these buttons in the empty state placeholders:
You have a message saying "add your first thing manually with the button below" but the actual button below opens the documentation. It was an oversight which should be corrected.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By the way, I think when those icons make their way in the navbar, we should replace these buttons in the empty state placeholders:
I hadn't thought of that, that's a good point.
You have a message saying "add your first thing manually with the button below" but the actual button below opens the documentation. It was an oversight which should be corrected.
Do you mean this?
The button is link to the docs, but the "click here" refers to the list item itself. Maybe that's not clear enough...
Otherwise, I'm not sure which button you're referring to.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By the way, I think when those icons make their way in the navbar, we should replace these buttons in the empty state placeholders:
Since we decided to just remove the button on narrow screens, I only removed the documentation buttons after the placeholders when the navbar help buttons are visible. So, on narrow screens instead of the navbar button you will still see the documentation buttons.
You have a message saying "add your first thing manually with the button below" but the actual button below opens the documentation. It was an oversight which should be corrected.
Do you mean this?
I'm still not 100% sure which message you are referring to, but I've added arrow indicators to all the click here
list items in the quickstart section which I hope will make it more clear that those specific items are clickable.
6c80f13
to
6d5b0eb
Compare
0d0f283
to
c791f31
Compare
Hi @JustinGeorgi, since it seems that @ghys is busy at the moment, I will take over the review of your PR, so stay tuned. |
Signed-off-by: jgeorgi <justin.georgi@gmail.com>
Signed-off-by: jgeorgi <justin.georgi@gmail.com>
Signed-off-by: jgeorgi <justin.georgi@gmail.com>
Signed-off-by: jgeorgi <justin.georgi@gmail.com>
Signed-off-by: jgeorgi <justin.georgi@gmail.com>
Signed-off-by: jgeorgi <justin.georgi@gmail.com>
Signed-off-by: jgeorgi <justin.georgi@gmail.com>
Signed-off-by: jgeorgi <justin.georgi@gmail.com>
Signed-off-by: jgeorgi <justin.georgi@gmail.com>
Signed-off-by: jgeorgi <justin.georgi@gmail.com>
Signed-off-by: jgeorgi <justin.georgi@gmail.com>
Signed-off-by: Justin Georgi <justin.georgi@gmail.com>
Signed-off-by: Justin Georgi <justin.georgi@gmail.com>
Signed-off-by: Justin Georgi <justin.georgi@gmail.com>
c791f31
to
8e8bab4
Compare
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks! Great addition.
I have rebased your PR, and I decided to directly commit my findings during review (this saves us both work, I anyway need to test some of my requested changes before giving a suggestion, then I'd have to wait for you implementing it, and finally test it again).
What I changed:
- You used
window.innerWidth
for the breakpoints. This works, but does not update when the window is resized. Better to use$f7.width
, because this is dynamic. - Added back a few documentationLinks that were removed, but still needed.
- Improved styling of the "Page Help" feature: The list items now look the same as for all other tabs.
- Added a cloud connector link to the remote access FAQ entry.
- Added a link to Thing add to the Quick Start entry.
In case one has suggestion regarding the content, let's merge this and then open a new PR. This PR is already big enough, no need to make it even bigger ;-)
No worries, I think that's a fine idea. Thanks for the review! |
Follow-up for openhab#2088. I forgot that during review, let's do it now. Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Follow-up for #2088. I forgot that during review, let's do it now. Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
) Follow-up for #2088. Depends on openhab/openhab-docs#2199. Also add the help sidebar button to the settings and list pages where it is missing. --------- Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Closes #2058.
This converts the developer sidebar into a multipurpose dock.
It keeps the previous developer sidebar as the
Tools
panel and adds aHelp
panel.The new
Help
panel contains 4 different sections:All of the main setting list pages and the overview page now have a help icon in the navbar that toggles the help dock.
FAQ and Quick-Start data are stored in separate JSON formatted files for ease of addition and maintenance.
Signed-off-by: Justin Georgi justin.georgi@gmail.com